WIP part of #161: Exploration player base 2 contentcard #125
Conversation
- Api to check dummy exploration
|
@BenHenning @rt4914 This PR is in progress I have issues related to databinding in adapter class. I have created custom adapter to support different viewtypes. This PR is to get clarity on whether the approach of fetching data is correct also need suggestions on it. PTAL |
|
@veena14cs can you merge your branch with latest develop before I have a look at your code. |
Yeah, okay |
BenHenning
changed the title
There was a problem hiding this comment.
Thanks @veena14cs! Generally, this approach looks good to me. I left a lot of comments, but quite a few are style based. Please do make sure though that all unused code is removed.
Could you also add tests for the new content card list fragment? And please let me know if you run into any issues--I've been having difficulties testing #172 in Robolectric and have been stuck on this for a couple of days.
app/src/main/java/org/oppia/app/player/content/ContentCardAdapter.kt
Outdated
Show resolved
Hide resolved
data/src/main/java/org/oppia/data/backends/gae/model/GaeState.kt
Outdated
Show resolved
Hide resolved
data/src/main/java/org/oppia/data/backends/gae/model/GaeSubtitledHtml.kt
Outdated
Show resolved
Hide resolved
BenHenning
assigned veena14cs and unassigned rt4914, BenHenning and nikitamarysolomanpvt
veena14cs
assigned BenHenning, rt4914 and nikitamarysolomanpvt and unassigned veena14cs
|
@veena14cs can you please merge this with latest |
|
@veena14cs also, remove FakeDataProvide if you are not using that file, because it has been removed from the |
There was a problem hiding this comment.
Thanks @veena14cs. I left some comments on the UrlImageParser, but at this point I think this PR is getting too large & doing too many things. Can we split up #161 such that:
- We're introducing the structure for exploration player & state (which is also fixing parts of UI Structure: StateCardFragment [Blocked: #122, #150, #161, #162] #163 and UI Structure: ExplorationPlayerFragment/Activity [Blocked: #163] #165)
- We're adding support for downloading and playing images
- We hook into ExplorationDataController & ExplorationProgressController to wrap up UI Structure: ContentCardView #161, UI Structure: StateCardFragment [Blocked: #122, #150, #161, #162] #163, and UI Structure: ExplorationPlayerFragment/Activity [Blocked: #163] #165?
If it doesn't slow things down, I suggest going farther and keeping the exploration/state/content card stuff all separate to have much smaller PRs.
| RecyclerView.Adapter<RecyclerView.ViewHolder>() { | ||
|
|
||
| private val VIEW_TYPE_CONTENT = 1 | ||
| private val VIEW_TYPE_RIGHT_INTERACTION = 2 |
There was a problem hiding this comment.
We shouldn't use positional language like 'left/right' since they may be reversed in RTL layouts.
Beyond that, I'm not sure I understand the directionality here. Per the mocks the interaction appears below the content area.
| <layout | ||
| xmlns:android="http://schemas.android.com/apk/res/android"> | ||
|
|
||
| <androidx.constraintlayout.widget.ConstraintLayout |
There was a problem hiding this comment.
Was this resolved? I don't see StateFragment being added anywhere.
| <layout | ||
| xmlns:android="http://schemas.android.com/apk/res/android"> | ||
|
|
||
| <androidx.constraintlayout.widget.ConstraintLayout |
There was a problem hiding this comment.
Ah, will those be added in this PR?
|
Duplicate PR of this is created here #189 |
Explanation
This PR includes rich-text handling in contentcard. It fetches contents from exploration API and extracts
html contentsfromstatesIssues